Skip to content

Enable ROR-Based Org Creation for Contributors#1243

Draft
momo3404 wants to merge 18 commits intointegrationfrom
momo/issues/926-edit
Draft

Enable ROR-Based Org Creation for Contributors#1243
momo3404 wants to merge 18 commits intointegrationfrom
momo/issues/926-edit

Conversation

@momo3404
Copy link
Collaborator

@momo3404 momo3404 commented Dec 5, 2025

Fixes #926.

Changes proposed in this PR:

  • Edit contributor affiliation dropdown partial to allow for search for orgs external to DMP Assistant DB.
  • Edit contributions_controller to process orgs that are external to DMP Assistant DB by validating ROR.
  • The workflow is as follows after a user enters an affiliation for a contributor:
    • If the affiliation already exists in the orgs table, the existing entry is used.
    • If the affiliation does not exist in the orgs table, an external API request to ROR is made. If a matching result is found, the new organisation is added to the orgs table along with the ror and fundref identifiers.
    • If the affiliation neither exists in the orgs table or on ROR, then there is no change made to the affiliation and an error message is displayed.

@momo3404 momo3404 self-assigned this Dec 5, 2025
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

1 Error
🚫 Please include a CHANGELOG entry. You can find it at CHANGELOG.md.

Generated by 🚫 Danger

Copy link
Collaborator

@aaronskiba aaronskiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a step forward, but I still can't help but wonder if moving the bulk of this logic inside of app/services/org_selection/hash_to_org_service.rb will simplify things even more.

You have 'shared/org_selectors/combined' in app/views/contributors/_form.html.erb, which is good; it should enable the ROR querying in the user search.

However, when I test it from integration it doesn't seem to work. It looks like the issue is the following:

      # app/services/external_apis/ror_service.rb
      def ping
        return true unless active? && heartbeat_path.present?

        resp = http_get(uri: "#{api_base_url}#{heartbeat_path}")
        byebug
        resp.present? && resp.code == 200
      end
(byebug) api_base_url
"https://api.ror.org/v1/"
(byebug) resp.present? && resp.code == 200
false
(byebug) resp
#<HTTParty::Response:0x10608 parsed_response={"errors"=>[{"status"=>"410", "title"=>"API Version Deprecated", "detail"=>"The v1 API has been deprecated. Please migrate to v2."}]}, @response=#<Net::HTTPGone 410 Gone readbody=true>, @headers={"date"=>["Tue, 09 Dec 2025 16:55:16 GMT"], "content-type"=>["application/json"], "content-length"=>["133"], "connection"=>["close"], "x-amzn-requestid"=>["23c106ef-fced-4488-a903-a468c6fd4d05"], "x-amzn-remapped-connection"=>["keep-alive"], "x-amz-apigw-id"=>["VVIiTH5ujoEEY0w="], "vary"=>["Origin"], "x-amzn-remapped-server"=>["nginx/1.18.0 + Phusion Passenger 6.0.7"], "status"=>["410 Gone"], "x-powered-by"=>["Phusion Passenger 6.0.7"], "x-amzn-remapped-date"=>["Tue, 09 Dec 2025 16:55:16 GMT"]}>
(byebug) 

According to https://ror.readme.io/, as of today, api v1 is no longer available:

As of December 9, 2025, version 1 of the ROR schema and API has been sunset, meaning that ROR API requests with v1 in the path no longer return results, v1 files will no longer be included in the ROR data dump, and v1 documentation is no longer available. Read more in our changelog.

@momo3404
Copy link
Collaborator Author

momo3404 commented Jan 6, 2026

This is a step forward, but I still can't help but wonder if moving the bulk of this logic inside of app/services/org_selection/hash_to_org_service.rb will simplify things even more.

I think moving parse_org_json, validate_ror, and existing_org_from_hash to HashToOrgService could work, what do you think?

@aaronskiba
Copy link
Collaborator

This is a step forward, but I still can't help but wonder if moving the bulk of this logic inside of app/services/org_selection/hash_to_org_service.rb will simplify things even more.

I think moving parse_org_json, validate_ror, and existing_org_from_hash to HashToOrgService could work, what do you think?

app/services/org_selection/hash_to_org_service.rb has the

You have 'shared/org_selectors/combined' in app/views/contributors/_form.html.erb, which is good; it should enable the ROR querying in the user search.

I'm still wondering if You have 'shared/org_selectors/combined' in app/views/contributors/_form.html.erb, which is good; it should enable the ROR querying in the user search.

This is a step forward, but I still can't help but wonder if moving the bulk of this logic inside of app/services/org_selection/hash_to_org_service.rb will simplify things even more.

I think moving parse_org_json, validate_ror, and existing_org_from_hash to HashToOrgService could work, what do you think?

I could be wrong, but after the work we did in app/services/external_apis/ror_service.rb, I'm still leaning toward re-architecting this so that the work is done in app/services/org_selection/hash_to_org_service.rb.

I'd try setting allow_create: true) in ContributorsController#process_org and keep your 'shared/org_selectors/combined' change in app/views/contributors/_form.html.erb, but from there, I'd try doing this work in the HashToOrgService.

I'm looking at HashToOrgService#to_org. It looks like it might already be doing a lot of the work that you've implemented directly in this controller?

The combined org partial allows for a search for orgs outside of the local DB, and includes external orgs with valid RORs
Updated process_org to parse org_id input and check for an existing Org
using the HashToOrgService before calling org_from_params. This avoids
redundant calls to org_from_params and speeds up the process of adding a contributor if their org already exists in the DB.

Also added finalize_org_hash helper method to avoid reuse of same code
- Add validate_ror helper function that makes an external API ROR request to ensure the external org picked from the dropdown has a valid ROR and that it is the correct one
- This allows valid external orgs with RORs to be added to the DB
- Also edit parse rescue result to nil since that is more readable
- The test stubs org_from_params to return new_org, but process_org may skip using new_org if it finds a different existing org based on org_name or parsed['id'].
- The fix makes the test avoid real org lookup by clearing values that might cause that. This ensures existing_org_from_hash returns nil, allowing the stubbed org_from_params to be used.
- Use the debounce function to create debouncedToggleWarning.
- Edit handleSelection to display the autocomplete warning only after 1 second of paused typing and hide it when user is typing
- Flash message displays when an org is not present in orgs table AND does not pass ROR validation
- Add request.present? to message condition in order for controller tests to work properly
@momo3404 momo3404 force-pushed the momo/issues/926-edit branch from 868ecf3 to 7ca17b2 Compare January 12, 2026 17:37
@momo3404 momo3404 marked this pull request as draft January 12, 2026 18:22
@momo3404 momo3404 force-pushed the momo/issues/926-edit branch from d5217a2 to 31fb8d9 Compare January 15, 2026 17:10
@aaronskiba
Copy link
Collaborator

Maybe something like Enable ROR-Based Org Creation for Contributors would be a better PR title if we ever have to lookup these changes in the future.

@momo3404
Copy link
Collaborator Author

Maybe something like Enable ROR-Based Org Creation for Contributors would be a better PR title if we ever have to lookup these changes in the future.

Sounds good, I can change it for the official PR once we're done with the draft one.

@momo3404 momo3404 changed the title Allow contributor affiliation to belong to external organisations Enable ROR-Based Org Creation for Contributors Jan 16, 2026
@aaronskiba
Copy link
Collaborator

aaronskiba commented Jan 19, 2026

Screenshot from 2026-01-19 10-32-38 I like the delay on the error message. But maybe it shouldn't pop up after only entering one or two characters?

EDIT: Maybe this behaviour existed prior to these changes?

@aaronskiba
Copy link
Collaborator

aaronskiba commented Jan 19, 2026

This PR will prevent users from adding "junk orgs" to the db. At the same time, we should try to not render those junk orgs in the dropdown list. For example, we probably don't want users to see all of these "test" options. We should try to figure out a way to filter out these unwanted orgs.
Screenshot from 2026-01-19 10-35-08

Edit: I might try to expand upon #1250. Maybe some more "junk orgs" could be removed via this rake task.

@aaronskiba
Copy link
Collaborator

When I add a non-db / non-ROR org, the contributor still gets saved to the db with org.id == nil. I also get Error: Invalid contributor affiliation. Please double check that your organisation does not appear in the list in a slightly different form.

The prior behaviour would also create the contributor with org.id == nil; maybe we want to preserve that behaviour like we are here. But if so, this feels confusing because the contributor creation succeeded, but the error message suggests something went wrong.

If we are maintaining the behaviour, maybe the flash message should be changed to something like the following:

flash[:notice] = _('Contributor saved without affiliation. If you intended to add an affiliation, please check if the organisation appears in the list in a different form.')

Screenshot from 2026-01-19 10-43-43 Screenshot from 2026-01-19 10-44-10

@momo3404
Copy link
Collaborator Author

momo3404 commented Jan 19, 2026

Screenshot from 2026-01-19 10-32-38 I like the delay on the error message. But maybe it shouldn't pop up after only entering one or two characters?
EDIT: Maybe this behaviour existed prior to these changes?

Should I add a limit then? Or what should be the behaviour be? The previous behavior had the error message showing up immediately after the first letter was typed.

@momo3404
Copy link
Collaborator Author

When I add a non-db / non-ROR org, the contributor still gets saved to the db with org.id == nil. I also get Error: Invalid contributor affiliation. Please double check that your organisation does not appear in the list in a slightly different form.

The prior behaviour would also create the contributor with org.id == nil; maybe we want to preserve that behaviour like we are here. But if so, this feels confusing because the contributor creation succeeded, but the error message suggests something went wrong.

If we are maintaining the behaviour, maybe the flash message should be changed to something like the following:

flash[:notice] = _('Contributor saved without affiliation. If you intended to add an affiliation, please check if the organisation appears in the list in a different form.')

I agree that the prior behaviour of saving the contributor should remain as is, I'll change the flash message.

@momo3404
Copy link
Collaborator Author

This PR will prevent users from adding "junk orgs" to the db. At the same time, we should try to not render those junk orgs in the dropdown list. For example, we probably don't want users to see all of these "test" options. We should try to figure out a way to filter out these unwanted orgs. Screenshot from 2026-01-19 10-35-08

Edit: I might try to expand upon #1250. Maybe some more "junk orgs" could be removed via this rake task.

I think a separate rake task to address this would be the best solution. Could be part of the next release with this and the funders PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants